Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clique Block Choice Rule #3436

Merged
merged 5 commits into from
Apr 2, 2021
Merged

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented Mar 26, 2021

Adds fork choice rules to clique aimed to prevent deadlocks.

Signed-off-by: Danno Ferrin danno.ferrin@gmail.com

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
Copy link
Contributor Author

shemnon commented Mar 29, 2021

@karalabe can I get your take?
@MicahZoltu can I get this merged?

Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay in getting to this, I have been incredibly busy the past couple weeks and have fallen behind.

None of the suggested changes are required for merge as draft, but some of them may become blockers for moving on to Review stage.


## Motivation

Recently (5 March 2021) there were a couple of deadlocks in the Goerli multi-client Clique network.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Recently (5 March 2021) there were a couple of deadlocks in the Goerli multi-client Clique network.
There has been more than one deadlock in the Goerli multi-client Clique network.

the **smallest** value.

```
(Hn - Vi) % Vt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend human readable variable names rather than single or double letter names:

Suggested change
(Hn - Vi) % Vt
(header_number - validator_index) % validator_count


## Reference Implementation

Implementation in Hyperledger Besu: https://github.com/hyperledger/besu/pull/2084
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference implementation should either be inline, in ../assets/eip-3436 or the entire section can be left out (it is optional). EIPs are not an appropriate place for lists of things or external links to third party repositories.

@MicahZoltu MicahZoltu merged commit e29ebb4 into ethereum:master Apr 2, 2021
phi-line pushed a commit to phi-line/EIPs that referenced this pull request Apr 29, 2021
Add a four step block rule to Clique that should reduce block production deadlocks
PhABC pushed a commit to PhABC/EIPs that referenced this pull request Jan 25, 2022
Add a four step block rule to Clique that should reduce block production deadlocks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants